Skip to content

Conversation

@chlowell
Copy link
Contributor

@chlowell chlowell commented May 18, 2023

This accessor encrypts data with DPAPI before writing it to a file. All platform specific implementations share the same constructor name, so creating an encrypting accessor looks the same on all platforms:

import "github.com/AzureAD/microsoft-authentication-extensions-for-go/extensions/cache/accessor"

// constructor parameters vary by implementation
a, err := accessor.New(...)
if err != nil {
	// TODO
}

Applications don't need to specify the platform because build constraints choose the right implementation at compile time. See #14 for an end-to-end example showing how accessors plug in to MSAL.

Closes #2

@chlowell chlowell marked this pull request as ready for review May 18, 2023 18:23
@jhendrixMSFT
Copy link

jhendrixMSFT commented May 18, 2023

Looking at the sample code in PR#14, I assume that one would replace a, err := file.New(path) with a, err := accessor.New(path) for an encrypted file, yes? Assuming that's the case, would it make more sense to call this package encryptedfile instead of accessor to clarify the distinction?

Maybe the idea is that we want folks to always use accessor, which is why the unencrypted implementation is a subpackage under accessor?

@chlowell
Copy link
Contributor Author

Assuming that's the case, would it make more sense to call this package encryptedfile instead of accessor to clarify the distinction?

The macOS and Linux implementations don't use a file for storage. There may be a better name for this though. My thinking with "accessor" is that it has everything that accesses storage, and accessor.New() builds the encrypting accessor for the platform.

Maybe the idea is that we want folks to always use accessor, which is why the unencrypted implementation is a subpackage under accessor?

That's the idea. Applications should prefer encrypted storage. The file package is a fallback for (typically Linux) applications that can't use our crypto dependency.

@jhendrixMSFT
Copy link

Agreed having "file" in the name doesn't make sense.

Is accessor always used with cache, or are there scenarios where it's independently used? If it's the former, what about placing it under the cache directory? Then the fully qualified name contains /cache/accessor.

@chlowell
Copy link
Contributor Author

That makes sense and I don't see a reason not to do it. Can't prevent applications running wild with exported types but the supported scenario is using accessor with cache.

@chlowell chlowell force-pushed the chlowell/windows branch from b5f9db7 to 43436dd Compare June 8, 2023 16:21
@chlowell chlowell merged commit 5ae5202 into dev Jun 8, 2023
@chlowell chlowell deleted the chlowell/windows branch June 8, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create a DP API Token Cache (Windows Only)

4 participants